Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent nvim_buf_get_name frees buf->b_ffname #89

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tony84727
Copy link

@tony84727 tony84727 commented Nov 5, 2022

nvim_buf_get_name C API doesn't copy memory but return a String with pointer to buf->b_ffname
rust shouldn't free buf->b_ffname

ref:
https://github.com/neovim/neovim/blob/09dffb9db7d16496e55e86f78ab60241533d86f6/src/nvim/api/buffer.c#L1038
https://github.com/neovim/neovim/blob/09dffb9db7d16496e55e86f78ab60241533d86f6/src/nvim/api/private/helpers.c#L403-L408

Reproduce code

use nvim_oxi as oxi;
use oxi::{Dictionary, Function};

#[oxi::module]
fn buffer_name_double_free_sample() -> oxi::Result<Dictionary> {
    Ok(Dictionary::from_iter([(
        "test",
        Function::from_fn(|()| {
            let buffer = oxi::api::get_current_buf();
            let name = buffer.get_name().unwrap();
            oxi::Result::<String>::Ok(String::from(name.to_str().unwrap()))
        }),
    )]))
}

Calling test function returned by the module twice will get a double free error (found on Mac)

nvim_buf_get_name C API doesn't copy memory but return
a String with pointer to buf->b_ffname
rust shouldn't free buf->b_ffname
@@ -118,8 +109,7 @@ extern "C" {
) -> Array;

// https://github.com/neovim/neovim/blob/master/src/nvim/api/buffer.c#L1086
pub(crate) fn nvim_buf_get_name(buf: BufHandle, err: *mut Error)
-> String;
pub(crate) fn nvim_buf_get_name(buf: BufHandle, err: *mut Error) -> Str;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a breaking change. Str might need to implement some traits. Maybe by duplicating some code from string.rs
The main different between Str and String is the former doesn't implement custom Drop to drop ffi memory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant